Skip to content

Fix style of the first ten solutions for Project Euler #3242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 25, 2020
Merged

Fix style of the first ten solutions for Project Euler #3242

merged 2 commits into from
Oct 25, 2020

Conversation

darkstar
Copy link
Contributor

Describe your change:

This changes the style of the first 10 Project Euler solutions akin to the changes requested in #2875 and #2880:

  • Unify the header docstring, and add reference URLs to Wikipedia or similar
  • Fix docstrings to be properly multilined
  • Add newlines where appropriate
  • Add doctests where they were missing
  • Remove doctests that test for the correct solution
  • Remove redundant (trivial) comments
  • fix obvious spelling or grammar mistakes in comments and exception messages
  • Fix line endings to be UNIX. This makes two of the files seem to have changed completely
  • no functional changes in any of the solutions were done (except for the spelling fixes mentioned above)
  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

@dhruvmanila Please comment if you have any input

@TravisBuddy
Copy link

Travis tests have failed

Hey @darkstar,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: 11067570-0cd7-11eb-b943-5b98ff54aa42

@TravisBuddy
Copy link

Travis tests have failed

Hey @darkstar,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: c7b13200-0cd8-11eb-b943-5b98ff54aa42

@darkstar
Copy link
Contributor Author

Okay, I had to remove the doctests from problem_08/sol3.py since they apparently give different results on Linux than they give on Windows. I literally copy/pasted what python gave me into the doctest, but travis seems to insist that the result is wrong...

Also I had to remove the prime test for 0 (which is not prime) since many solutions didn't properly check for that and I didn't want to change the implementation

@dhruvmanila
Copy link
Member

We really appreciate you doing this but the guidelines for maintaining consistency in Project Euler are still in progress. So I will keep this PR on hold and convert to draft. In a few days once that guideline is out you can read it through and make the necessary changes where applicable.

@dhruvmanila dhruvmanila marked this pull request as draft October 13, 2020 01:16
dhruvmanila
dhruvmanila reviewed Oct 15, 2020
View reviewed changes
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Announcement:

@darkstar

This message is sent to all pull requests which are OPEN, NOT MERGED, not containing 'wontfix' label and contains 'Project Euler' or 'Project' or 'Euler' (case doesn't matter) in their title. If this message doesn't apply to your pull request, please ignore this.

Message:

This is to notify all the users submitting a pull request for the Project Euler solution that the README.md file has been updated with all the style requirements necessary for the directory and solution files. Please read through it and make all the necessary changes required to your submission.

@dhruvmanila dhruvmanila added the awaiting changes A maintainer has requested changes to this PR label Oct 16, 2020
@darkstar
Fix style of the first ten solutions for Project Euler
3e7ee2c 
- Unify the header docstring, and add reference URLs to wikipedia
  or similar
- Fix docstrings to be properly multilined
- Add newlines where appropriate
- Add doctests where they were missing
- Remove doctests that test for the correct solution
- fix obvious spelling or grammar mistakes in comments and
  exception messages
- Fix line endings to be UNIX. This makes two of the files seem
  to have changed completely
- no functional changes in any of the solutions were done
  (except for the spelling fixes mentioned above)
@darkstar darkstar marked this pull request as ready for review October 16, 2020 22:18
@darkstar
Copy link
Contributor Author

I fixed all issues that were not conforming to the styleguide.

This is still only for the first 10 projects but once this gets committed I'll gladly do the rest too

There is still the issue of the broken main function though, I'm not sure how you want it fixed as the bug is in the style guide's template. But once that is fixed in the guide, I'll incorporate it into the PR

@TravisBuddy
Copy link

Travis tests have failed

Hey @darkstar,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: 437282e0-0ffe-11eb-8acb-27a1e4e1eb88

@dhruvmanila
Copy link
Member

Can you specify what the bug is in the template?

@dhruvmanila dhruvmanila added enhancement This PR modified some existing files and removed on hold labels Oct 24, 2020
@darkstar
Copy link
Contributor Author

Can you specify what the bug is in the template?

I posted it here: 2d7e08e#comments

This code does not work:

if __name__ == "__main__":
    print(f"{solution() = }")

it gives a syntax error when the file is run through python:

  File "<fstring>", line 1
    (solution() = )
                ^
SyntaxError: invalid syntax

But I left it in anyway since it might just be my setup (Windows-related, python 3.6.3 related, whatever)

@dhruvmanila
Copy link
Member

dhruvmanila commented Oct 25, 2020 •
edited
Loading

That might be the version problem. Which version of Python are you running?

$ python -V

This repository assumes you're on the latest version of Python.

dhruvmanila
dhruvmanila approved these changes Oct 25, 2020
View reviewed changes
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! Thanks for doing this :)

@dhruvmanila dhruvmanila merged commit 98e9d6b into TheAlgorithms:master Oct 25, 2020
@dhruvmanila dhruvmanila removed awaiting changes A maintainer has requested changes to this PR enhancement This PR modified some existing files labels Oct 25, 2020
@darkstar
Copy link
Contributor Author

That might be the version problem. Which version of Python are you running?

$ python -V

This repository assumes you're on the latest version of Python.

3.6.3

I'll be updating to see if that helps. I thought 3.6 was recent enough, but now I see we're already at 3.9, so yeah, probably a problem on my side

stokhos pushed a commit to stokhos/Python that referenced this pull request Jan 3, 2021
@darkstar @stokhos
Fix style of the first ten solutions for Project Euler (TheAlgorithms…
9f39a05 
…#3242)

* Fix style of the first ten solutions for Project Euler

- Unify the header docstring, and add reference URLs to wikipedia
  or similar
- Fix docstrings to be properly multilined
- Add newlines where appropriate
- Add doctests where they were missing
- Remove doctests that test for the correct solution
- fix obvious spelling or grammar mistakes in comments and
  exception messages
- Fix line endings to be UNIX. This makes two of the files seem
  to have changed completely
- no functional changes in any of the solutions were done
  (except for the spelling fixes mentioned above)

* Fix docstrings and main function as per Style Guide
peRFectBeliever pushed a commit to peRFectBeliever/Python that referenced this pull request Apr 1, 2021
@darkstar @peRFectBeliever
Fix style of the first ten solutions for Project Euler (TheAlgorithms…
4e8aabd 
…#3242)

* Fix style of the first ten solutions for Project Euler

- Unify the header docstring, and add reference URLs to wikipedia
  or similar
- Fix docstrings to be properly multilined
- Add newlines where appropriate
- Add doctests where they were missing
- Remove doctests that test for the correct solution
- fix obvious spelling or grammar mistakes in comments and
  exception messages
- Fix line endings to be UNIX. This makes two of the files seem
  to have changed completely
- no functional changes in any of the solutions were done
  (except for the spelling fixes mentioned above)

* Fix docstrings and main function as per Style Guide
Panquesito7 pushed a commit to Panquesito7/Python that referenced this pull request May 13, 2021
@darkstar
Fix style of the first ten solutions for Project Euler (TheAlgorithms…
47356b9 
…#3242)

* Fix style of the first ten solutions for Project Euler

- Unify the header docstring, and add reference URLs to wikipedia
  or similar
- Fix docstrings to be properly multilined
- Add newlines where appropriate
- Add doctests where they were missing
- Remove doctests that test for the correct solution
- fix obvious spelling or grammar mistakes in comments and
  exception messages
- Fix line endings to be UNIX. This makes two of the files seem
  to have changed completely
- no functional changes in any of the solutions were done
  (except for the spelling fixes mentioned above)

* Fix docstrings and main function as per Style Guide
shermanhui pushed a commit to shermanhui/Python that referenced this pull request Oct 22, 2021
@darkstar @shermanhui
Fix style of the first ten solutions for Project Euler (TheAlgorithms…
9f4c437 
…#3242)

* Fix style of the first ten solutions for Project Euler

- Unify the header docstring, and add reference URLs to wikipedia
  or similar
- Fix docstrings to be properly multilined
- Add newlines where appropriate
- Add doctests where they were missing
- Remove doctests that test for the correct solution
- fix obvious spelling or grammar mistakes in comments and
  exception messages
- Fix line endings to be UNIX. This makes two of the files seem
  to have changed completely
- no functional changes in any of the solutions were done
  (except for the spelling fixes mentioned above)

* Fix docstrings and main function as per Style Guide
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhruvmanila dhruvmanila dhruvmanila approved these changes

@Kush1101 Kush1101 Awaiting requested review from Kush1101

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@darkstar @TravisBuddy @dhruvmanila